[RAC][Security Solution] Add base Security Rule Type by madirey · Pull Request #105096 · elastic/kibana

madirey · 2021-07-09T16:54:30Z

Summary

This PR adds a base security rule type, which wraps the persistence rule type to provide Detections-specific logic.

Summary of changes

Add base security rule type
- Implement gap detection
- Do privilege checks
- Handle exception lists
- Map event fields to new alerts-as-data schema (outlined here)
- End-to-end query rule execution
- Static typing for new rule executors

How to test this implementation

Enable experimental feature flag: echo "xpack.securitySolution.enableExperimental: ['ruleRegistryEnabled']" >> ./config/kibana.dev.yml
Enable rule registry writing: echo "xpack.ruleRegistry.write.enabled: true" >> ./config/kibana.dev.yml
Create a rule by running ./x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/scripts/create_rule_query.sh

It creates a rule that generates up to 10 alerts every minute or so. The created rule is not visible in our UI and is not accessible through most of our API endpoints.

To be addressed in future PRs

Implementations for the remaining 4 rule types
Remove threshold rule's reliance on outputIndex by utilizing state

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

…076ca54526ea0e61a9a99e1c1bce854806977e

…t tests

…-type

…nd daterange tuple

…-type

dominiqueclarke

Uptime changes look good to me. Only did a code review.

dominiqueclarke · 2021-07-30T16:24:08Z

-      dateRangeStart: moment(new Date(fields['kibana.rac.alert.start']!))
-        .subtract('5', 'm')
-        .toISOString(),
+      dateRangeEnd: fields[ALERT_STATUS] === 'open' ? 'now' : fields[ALERT_END]!,


Thank you for doing this. I actually have a branch up getting ready to update this. Much appreciated.

xcrzx

Checked changes in regards to rule execution logging, LGTM 👍
It would be great to merge this PR sooner so that we can start integration with the new Exec log.

dhurley14

In regards to the comment in your PR description around The created rule is not visible in our UI and is not accessible through most of our API endpoints. I think we just need a small update so we can still manage these rules through the security solution detections page by updating the routes to include the new QUERY_ALERT_TYPE_ID.

One example is in the find_rules route

kibana/x-pack/plugins/security_solution/server/lib/detection_engine/rules/find_rules.ts

Lines 13 to 19 in 01293bf

    
           export const getFilter = (filter: string | null | undefined) => { 
        
             if (filter == null) { 
        
               return `alert.attributes.alertTypeId: ${SIGNALS_ID}`; 
        
             } else { 
        
               return `alert.attributes.alertTypeId: ${SIGNALS_ID} AND ${filter}`; 
        
             } 
        
           };

where we can update this filter to include the new QUERY_ALERT_TYPE_ID

If you agree with the above, I think there are other places where this change should propagate as well, like in the import rules route.

ecezalp

Wow! The changes look fantastic. I think that the overall modularization efforts really came together, and everything became easier to understand! Thank you so much for doing this work!

One comment though - I flipped on the ruleRegistryFlag (by adding xpack.securitySolution.enableExperimental: ['ruleRegistryEnabled'] to my kibana.dev.yml, but I wasn't able to get any alerts on the Custom Query Rule. It seemed like a new alerting index didn't get created, I wasn't able to catch any suspicious errors / logs in my terminal. I was wondering if I am missing a step or if I should try again. Please let me know! just noticed the instructions above. will try again!

…-type

michaelolo24 · 2021-08-02T14:34:47Z

+    array: false,
+    required: true,
+  },
+  'kibana.alert.threat': {


making a personal note to add kibana.alert.reason 😄

madirey · 2021-08-02T15:04:08Z

@elasticmachine merge upstream

madirey · 2021-08-02T16:57:30Z

@elasticmachine merge upstream

…-type

…ecurity-rule-type

…-type

smith

APM changes look good.

kibanamachine · 2021-08-03T16:48:04Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	2338	2341	+3
`uptime`	563	566	+3
total			+6

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`apm`	4.3MB	4.3MB	+13.7KB
`observability`	490.9KB	507.4KB	+16.5KB
`securitySolution`	6.4MB	6.4MB	+5.8KB
`uptime`	954.5KB	954.5KB	+8.0B
total			+36.1KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`apm`	41.6KB	44.4KB	+2.8KB
`infra`	146.4KB	149.2KB	+2.8KB
`uptime`	29.3KB	34.7KB	+5.4KB
total			+10.9KB

Unknown metric groups

API count

id	before	after	diff
`ruleRegistry`	82	89	+7

API count missing comments

id	before	after	diff
`ruleRegistry`	82	89	+7

References to deprecated APIs

id	before	after	diff
`securitySolution`	836	832	-4

History

💔 Build #142648 failed 9c02627
💚 Build #142389 succeeded cfbdd20
💔 Build #142331 failed 3ef3de9
💛 Build #142106 was flaky 7f519e8
💚 Build #142053 succeeded 44d0a2a

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* injects bulkCreate and wrapHits to individual rule executors * WIP create_security_rule_type_factory based on Marshall's work in #d3076ca54526ea0e61a9a99e1c1bce854806977e * removes ruleStatusService from old rule executors, fixes executor unit tests * fixes rebase * Rename reference_rules to rule_types * Fix type errors * Fix type errors in base security rule factory * Additional improvements to types and interfaces * More type alignment * Fix remaining type errors in query rule * Add validation / inject lists plugin * Formatting * Improvements to typing * Static typing on executors * cleanup * Hook up params for query/threshold rules... includes exceptionsList and daterange tuple * Scaffolding for wrapHits and bulkCreate * Add error handling / status reporting * Fixup alert type state * Begin threshold * Begin work on threshold state * Organize rule types * Export base security rule types * Fixup lifecycle static typing * WrapHits / bulk changes * Field mappings (partial) * whoops * Remove redundant params * More flexibile implementation of bulkCreateFactory * Add mappings * Finish query rule * Revert "Remove redundant params" This reverts commit 87aff9c. * Revert "whoops" This reverts commit a7771bd. * Fixup return types * Use alertWithPersistence * Fix import * End-to-end rule mostly working * Fix bulkCreate * Bug fixes * Bug fixes and mapping changes * Fix indexing * cleanup * Fix type errors * Test fixes * Fix query tests * cleanup / rename kibana.rac to kibana * Remove eql/threshold (for now) * Move technical fields to package * Add indexAlias and buildRuleMessageFactory * imports * type errors * Change 'kibana.rac.*' to 'kibana.*' * Fix lifecycle tests * Single alert instance * fix import * Fix type error * Fix more type errors * Fix query rule type test * revert to previous ts-expect-error * type errors again * types / linting * General readability improvements * Add invariant function from Dmitrii's branch * Use invariant and constants * Improvements to field mappings * More test failure fixes * Add refresh param for bulk create * Update more field refs * Actually use refresh param * cleanup * test fixes * changes to rule creation script * Fix created signals count * Use ruleId * Updates to bulk indexing * Mapping updates * Cannot use 'strict' for dynamic setting Co-authored-by: Marshall Main <marshall.main@elastic.co> Co-authored-by: Ece Ozalp <ozale272@newschool.edu> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* injects bulkCreate and wrapHits to individual rule executors * WIP create_security_rule_type_factory based on Marshall's work in #d3076ca54526ea0e61a9a99e1c1bce854806977e * removes ruleStatusService from old rule executors, fixes executor unit tests * fixes rebase * Rename reference_rules to rule_types * Fix type errors * Fix type errors in base security rule factory * Additional improvements to types and interfaces * More type alignment * Fix remaining type errors in query rule * Add validation / inject lists plugin * Formatting * Improvements to typing * Static typing on executors * cleanup * Hook up params for query/threshold rules... includes exceptionsList and daterange tuple * Scaffolding for wrapHits and bulkCreate * Add error handling / status reporting * Fixup alert type state * Begin threshold * Begin work on threshold state * Organize rule types * Export base security rule types * Fixup lifecycle static typing * WrapHits / bulk changes * Field mappings (partial) * whoops * Remove redundant params * More flexibile implementation of bulkCreateFactory * Add mappings * Finish query rule * Revert "Remove redundant params" This reverts commit 87aff9c. * Revert "whoops" This reverts commit a7771bd. * Fixup return types * Use alertWithPersistence * Fix import * End-to-end rule mostly working * Fix bulkCreate * Bug fixes * Bug fixes and mapping changes * Fix indexing * cleanup * Fix type errors * Test fixes * Fix query tests * cleanup / rename kibana.rac to kibana * Remove eql/threshold (for now) * Move technical fields to package * Add indexAlias and buildRuleMessageFactory * imports * type errors * Change 'kibana.rac.*' to 'kibana.*' * Fix lifecycle tests * Single alert instance * fix import * Fix type error * Fix more type errors * Fix query rule type test * revert to previous ts-expect-error * type errors again * types / linting * General readability improvements * Add invariant function from Dmitrii's branch * Use invariant and constants * Improvements to field mappings * More test failure fixes * Add refresh param for bulk create * Update more field refs * Actually use refresh param * cleanup * test fixes * changes to rule creation script * Fix created signals count * Use ruleId * Updates to bulk indexing * Mapping updates * Cannot use 'strict' for dynamic setting Co-authored-by: Marshall Main <marshall.main@elastic.co> Co-authored-by: Ece Ozalp <ozale272@newschool.edu> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Marshall Main <marshall.main@elastic.co> Co-authored-by: Ece Ozalp <ozale272@newschool.edu> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* injects bulkCreate and wrapHits to individual rule executors * WIP create_security_rule_type_factory based on Marshall's work in #d3076ca54526ea0e61a9a99e1c1bce854806977e * removes ruleStatusService from old rule executors, fixes executor unit tests * fixes rebase * Rename reference_rules to rule_types * Fix type errors * Fix type errors in base security rule factory * Additional improvements to types and interfaces * More type alignment * Fix remaining type errors in query rule * Add validation / inject lists plugin * Formatting * Improvements to typing * Static typing on executors * cleanup * Hook up params for query/threshold rules... includes exceptionsList and daterange tuple * Scaffolding for wrapHits and bulkCreate * Add error handling / status reporting * Fixup alert type state * Begin threshold * Begin work on threshold state * Organize rule types * Export base security rule types * Fixup lifecycle static typing * WrapHits / bulk changes * Field mappings (partial) * whoops * Remove redundant params * More flexibile implementation of bulkCreateFactory * Add mappings * Finish query rule * Revert "Remove redundant params" This reverts commit 87aff9c. * Revert "whoops" This reverts commit a7771bd. * Fixup return types * Use alertWithPersistence * Fix import * End-to-end rule mostly working * Fix bulkCreate * Bug fixes * Bug fixes and mapping changes * Fix indexing * cleanup * Fix type errors * Test fixes * Fix query tests * cleanup / rename kibana.rac to kibana * Remove eql/threshold (for now) * Move technical fields to package * Add indexAlias and buildRuleMessageFactory * imports * type errors * Change 'kibana.rac.*' to 'kibana.*' * Fix lifecycle tests * Single alert instance * fix import * Fix type error * Fix more type errors * Fix query rule type test * revert to previous ts-expect-error * type errors again * types / linting * General readability improvements * Add invariant function from Dmitrii's branch * Use invariant and constants * Improvements to field mappings * More test failure fixes * Add refresh param for bulk create * Update more field refs * Actually use refresh param * cleanup * test fixes * changes to rule creation script * Fix created signals count * Use ruleId * Updates to bulk indexing * Mapping updates * Cannot use 'strict' for dynamic setting Co-authored-by: Marshall Main <marshall.main@elastic.co> Co-authored-by: Ece Ozalp <ozale272@newschool.edu> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

marshallmain and others added 14 commits June 30, 2021 12:12

injects bulkCreate and wrapHits to individual rule executors

e2467ac

WIP create_security_rule_type_factory based on Marshall's work in #d3…

ae41613

…076ca54526ea0e61a9a99e1c1bce854806977e

removes ruleStatusService from old rule executors, fixes executor uni…

83a2f0f

…t tests

fixes rebase

5fd3f60

Merge branch 'master' of github.com:elastic/kibana into security-rule…

b95b6b6

…-type

Rename reference_rules to rule_types

2c0236e

Fix type errors

637246c

Fix type errors in base security rule factory

c336531

Additional improvements to types and interfaces

0645902

More type alignment

72032d7

Fix remaining type errors in query rule

7ea0928

Add validation / inject lists plugin

c92dbe6

Merge branch 'master' of github.com:elastic/kibana into security-rule…

849a428

…-type

Formatting

6fb0fc8

madirey added 2 commits July 9, 2021 14:42

Improvements to typing

b302674

Static typing on executors

3cd9ee7

madirey requested review from ecezalp and marshallmain July 9, 2021 21:18

madirey added 3 commits July 11, 2021 10:02

cleanup

44eb2de

Hook up params for query/threshold rules... includes exceptionsList a…

b4b7b56

…nd daterange tuple

Merge branch 'master' of github.com:elastic/kibana into security-rule…

14b0b6b

…-type

madirey added 2 commits July 30, 2021 11:36

Fix created signals count

01529fc

Use ruleId

668dddc

dominiqueclarke approved these changes Jul 30, 2021

View reviewed changes

madirey added 2 commits July 30, 2021 13:44

Updates to bulk indexing

8d19387

Mapping updates

44d0a2a

xcrzx approved these changes Jul 30, 2021

View reviewed changes

dhurley14 reviewed Jul 30, 2021

View reviewed changes

ecezalp approved these changes Jul 30, 2021

View reviewed changes

madirey added 2 commits July 30, 2021 19:54

Cannot use 'strict' for dynamic setting

091b64a

Merge branch 'master' of github.com:elastic/kibana into security-rule…

7f519e8

…-type

michaelolo24 reviewed Aug 2, 2021

View reviewed changes

Merge branch 'master' into security-rule-type

3ef3de9

Merge branch 'master' into security-rule-type

cfbdd20

michaelolo24 approved these changes Aug 2, 2021

View reviewed changes

madirey enabled auto-merge (squash) August 3, 2021 03:37

mgiota mentioned this pull request Aug 3, 2021

[Observability RAC] change alerts table filter text box placeholder #107366

Merged

madirey added 4 commits August 3, 2021 10:06

Merge branch 'master' of github.com:elastic/kibana into security-rule…

75260cf

…-type

Merge branch 'security-rule-type' of github.com:madirey/kibana into s…

a4b37b0

…ecurity-rule-type

Merge branch 'master' of github.com:elastic/kibana into security-rule…

9c02627

…-type

Fix type errors from master

1882710

smith approved these changes Aug 3, 2021

View reviewed changes

madirey merged commit 8f9086b into elastic:master Aug 3, 2021

michaelolo24 mentioned this pull request Aug 5, 2021

[Security Solution][RAC] - Update UI signal references #107713

Closed

madirey mentioned this pull request Aug 5, 2021

[7.x] [RAC][Security Solution] Add base Security Rule Type (#105096) #107755

Merged

	export const getFilter = (filter: string \| null \| undefined) => {
	if (filter == null) {
	return `alert.attributes.alertTypeId: ${SIGNALS_ID}`;
	} else {
	return `alert.attributes.alertTypeId: ${SIGNALS_ID} AND ${filter}`;
	}
	};

Conversation

madirey commented Jul 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Summary of changes

How to test this implementation

To be addressed in future PRs

Checklist

Risk Matrix

For maintainers

Uh oh!

dominiqueclarke left a comment

Choose a reason for hiding this comment

Uh oh!

dominiqueclarke Jul 30, 2021

Choose a reason for hiding this comment

Uh oh!

xcrzx left a comment

Choose a reason for hiding this comment

Uh oh!

dhurley14 left a comment

Choose a reason for hiding this comment

Uh oh!

ecezalp left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

michaelolo24 Aug 2, 2021

Choose a reason for hiding this comment

Uh oh!

madirey commented Aug 2, 2021

Uh oh!

madirey commented Aug 2, 2021

Uh oh!

smith left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Aug 3, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

API count

API count missing comments

References to deprecated APIs

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

madirey commented Jul 9, 2021 •

edited

Loading

ecezalp left a comment •

edited

Loading